Skip to content

fix(file-dropzone): fix multi-folder drop losing all but first folder#1340

Closed
iOvergaard wants to merge 1 commit intomainfrom
bugfix/file-dropzone-datatransfer-staleness
Closed

fix(file-dropzone): fix multi-folder drop losing all but first folder#1340
iOvergaard wants to merge 1 commit intomainfrom
bugfix/file-dropzone-datatransfer-staleness

Conversation

@iOvergaard
Copy link
Copy Markdown
Contributor

Summary

Port of #1339 (targeting v1/dev) to main for v2 tracking.

Fixes the DataTransfer staleness bug in uui-file-dropzone that caused only the first dropped folder to be processed when multiple folders were dragged and dropped simultaneously.

See #1339 for the full description, root cause analysis, and Mermaid diagrams.

Test plan

… first await

DataTransferItem.webkitGetAsEntry() returns null after the first await
because the browser expires the drag data store. By collecting all
FileSystemEntry references in a synchronous pass first, all dropped
folders are processed — not just the first one.

Also removes _processFileEntry which used DataTransferItem.getAsFile()
(same staleness issue), replaced by FileSystemFileEntry.file().
Copilot AI review requested due to automatic review settings March 9, 2026 10:32
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 9, 2026

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 9, 2026

Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-beach-055ecb503-1340.westeurope.azurestaticapps.net

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ports the v1 fix for uui-file-dropzone to main (v2), addressing a browser DataTransfer staleness issue that caused multi-folder drops to only process the first folder by collecting FileSystemEntry references synchronously before any await.

Changes:

  • Refactors _getAllEntries into a synchronous “collect refs” phase plus an async _processRootEntries phase to avoid DataTransferItem.webkitGetAsEntry() staleness.
  • Replaces root-level DataTransferItem.getAsFile() handling with FileSystemFileEntry.file() via _getAsFile.
  • Adds 4 unit tests for _processRootEntries covering folders/files and accept/reject behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/components/file-dropzone/file-dropzone.element.ts Refactors drop processing to avoid DataTransfer invalidation and supports root-level files/folders via stable FileSystemEntry refs.
src/components/file-dropzone/file-dropzone.test.ts Adds unit tests for the new _processRootEntries behavior.
Comments suppressed due to low confidence (1)

src/components/file-dropzone/file-dropzone.element.ts:158

  • _getEntry now returns FileSystemEntry | null, but the local variable is still typed/named as a directory (let dir: FileSystemDirectoryEntry | null) and the result of webkitGetAsEntry() is cast to FileSystemDirectoryEntry. This is now type-incorrect (root items can be files) and undermines type safety for callers. Update the local variable/casts to use FileSystemEntry | null (and consider renaming dir), so the implementation matches the new return type.
  private _getEntry(entry: DataTransferItem): FileSystemEntry | null {
    let dir: FileSystemDirectoryEntry | null = null;

    if ('webkitGetAsEntry' in entry) {
      dir = entry.webkitGetAsEntry() as FileSystemDirectoryEntry;

@@ -165,7 +151,7 @@ export class UUIFileDropzoneElement extends LabelMixin('', LitElement) {
* Get the directory entry from a DataTransferItem.
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc for _getEntry still says it returns a "directory entry", but the method now returns FileSystemEntry (can be a file or directory). Updating the doc comment/wording would avoid confusion for future readers.

Suggested change
* Get the directory entry from a DataTransferItem.
* Get the FileSystemEntry (file or directory) from a DataTransferItem.

Copilot uses AI. Check for mistakes.
@iOvergaard
Copy link
Copy Markdown
Contributor Author

Superseded by the forward-merge of v1/dev into main.

@iOvergaard iOvergaard closed this Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants